Skip to content

[#231] Validate plot existence before allowing comments#236

Merged
realproject7 merged 2 commits intomainfrom
task/231-comment-plot-validation
Mar 17, 2026
Merged

[#231] Validate plot existence before allowing comments#236
realproject7 merged 2 commits intomainfrom
task/231-comment-plot-validation

Conversation

@realproject7
Copy link
Copy Markdown
Owner

Summary

  • POST /api/comments now queries the plots table to verify the (storyline_id, plot_index, contract_address) tuple exists before inserting
  • Returns 400 "Plot does not exist" if the plot is not found
  • Check runs after signature verification but before rate limiting (no DB writes wasted on invalid plots)

Test plan

  • POST comment on valid plot → succeeds
  • POST comment on non-existent plot_index → returns 400
  • POST comment on non-existent storyline_id → returns 400

Fixes #231

🤖 Generated with Claude Code

POST /api/comments now returns 400 if the (storyline_id, plot_index)
pair doesn't exist in the plots table, preventing comments on
non-existent plot indexes.

Fixes #231

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Collaborator

@project7-interns project7-interns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: REQUEST CHANGES

Summary

The new plot-existence validation covers the happy path, but it currently drops database errors from the lookup and misreports them as a client-side 400. That turns transient/server failures into a false "plot does not exist" response.

Findings

  • [medium] The plots existence query ignores its error result, so any lookup failure is treated as a missing plot instead of a server error.
    • File: src/app/api/comments/route.ts:117
    • Suggestion: capture error from the serverClient.from("plots")... query and return error(Database error: ${dbError.message}, 500) before the not-found check.

Decision

Requesting changes because the new validation path currently hides real database failures behind a misleading 400 response.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Collaborator

@project7-interns project7-interns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: APPROVE

Summary

Re-review complete. The plot-existence validation now correctly handles database lookup failures as 500 responses and only returns 400 when the plot tuple is genuinely missing.

Findings

  • None. The validation path now matches issue #231's acceptance criteria without masking server-side failures.

Decision

Approving because the requested fix is in place and the route behavior now cleanly distinguishes invalid input from database errors. CI was still pending at the time of review.

@realproject7 realproject7 merged commit 6268df2 into main Mar 17, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Tech Debt] Add FK constraint on comments.plot_index

2 participants